don't fail when chown to postgres fails #888
Conversation
evgeni
commented
Jun 25, 2024
evgeni
commented
- add prepare directory tests
- move chown postgres inside unless preserve
- only chown to postgres group for online backups with local DB
- don't fail when chown to postgres fails
all others do not need the postgres user to be able to read the backup for restore
# foreman-maintain backup online /mnt/online-new-1
Starting backup: 2024-06-25 09:11:30 +0000
…
--------------------------------------------------------------------------------
Prepare backup Directory:
Creating backup folder /mnt/online-new-1/katello-backup-2024-06-25-09-11-30
[WARNING]
/mnt/online-new-1/katello-backup-2024-06-25-09-11-30 could not be made readable by the 'postgres' user.
This won't affect the backup procedure, but you have to ensure that
the 'postgres' user can read the data during restore.
--------------------------------------------------------------------------------
…
--------------------------------------------------------------------------------
Scenario [Backup] failed.
The following steps ended up in warning state:
[backup-prepare-directory]
The steps in warning state itself might not mean there is an error,
but it should be reviewed to ensure the behavior is expected
Done with backup: 2024-06-25 09:11:39 +0000
**** BACKUP Complete, contents can be found in: /mnt/online-new-1/katello-backup-2024-06-25-09-11-30 ****
# echo $?
78 |
| MSG | ||
| set_status(:warning, warn_msg) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| FileUtils.rm(Dir.glob(File.join(@backup_dir, '.*.snar'))) if @preserve_dir |
There was a problem hiding this comment.
Should this be in the else part of unless @preserve_dir?
There was a problem hiding this comment.
Maybe. But honestly I didn't want to touch too much of the code, just the parts that I actually wanted to change.
Also, I am not really sure why this is guarded by if @preserve_dir anyway.
As when it's false, the @backup_dir will be fresh and empty and the .glob will just return [].
| puts "Creating backup folder #{@backup_dir}" | ||
|
|
||
| unless @preserve_dir |
There was a problem hiding this comment.
I think this would actually make a lot more sense, but I can understand why you don't want to touch those too much
| puts "Creating backup folder #{@backup_dir}" | |
| unless @preserve_dir | |
| if @preserve_dir | |
| FileUtils.rm(Dir.glob(File.join(@backup_dir, '.*.snar'))) | |
| else | |
| puts "Creating backup folder #{@backup_dir}" |
There was a problem hiding this comment.
(moved the puts, but not the rm)
I think what my brain tries to fight is the fact that "not preserve_dir" is the primary mode of operation of this code, so it should come first vs the else branch that is, well, "else" ;-)
